Skip to content

fix(mobile): bind same-name mentions to exact selected identities - #7385

Open
loganj wants to merge 5 commits into
mainfrom
fix/mobile-exact-mentions-20260905
Open

fix(mobile): bind same-name mentions to exact selected identities#7385
loganj wants to merge 5 commits into
mainfrom
fix/mobile-exact-mentions-20260905

Conversation

@loganj

@loganj loganj commented Sep 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

🤖

Summary

If two people in a channel share a display name, mobile couldn't tell them apart in mentions: picking the second could overwrite the first's selection, a rendered mention linked whichever same-name person matched first, and a later rename or a shorter name could re-bind the text to the wrong recipient. This PR binds every mention to the exact selected identity:

  • Each same-name selection keeps its own recipient instead of overwriting by name; conflicting picks get a qualified label like Name (key…).
  • The longest matching label wins, so a shorter or interior name can never claim part of a longer one and steal its identity.
  • Rendering resolves recipients by the signed identity key from the event's tags — never from message text alone — so qualified labels stay correct regardless of tag order and survive later renames; an untagged ambiguous label blocks shorter mentions instead of silently re-binding.

Ports the landed Desktop exact-recipient behavior (see docs/mention-editor.md).

Related issue

  • Fixes: N/A. No mobile issue; Desktop's landed exact-recipient fixes are the reference this ports.
  • Independent base (main). fix(mobile): preserve exact mention recipients in saved drafts #7387 (child) persists these exact selections in saved drafts.
  • Landing note: branches in this series overlap in the composer — when rebasing, keep exact/durable mention bindings, the explicit invite/reference-only choice, the account/visit/revision fences, and authorization before membership preparation and publication; don't resolve conflicts by taking either side wholesale.
  • Draft — not requesting merge yet; the security advisory run for this range timed out without results (no verdict).

Testing

  • Regressions cover same-name collisions, prefix/overlap, removal, tag order, and renames.
  • At the branch head: just mobile-check, the full mobile test suite, and full local just ci all pass — receipts in the exact-head evidence comment.
  • Verification is widget-test level; no native device or simulator run is claimed.

To see it: mention two teammates with the same display name — both stay distinct, the second shows a qualified label, and the rendered message keeps both correct even after either renames.

Screenshots

Flutter production-widget test renders — not native-device screenshots or acceptance captures.

Scenario Before After
Qualified mention chip for same-name recipients, at 200% text scale (deliberate stress fixture) Before: the short chip is followed by the distinguishing key spilling out as raw text After: the full qualified label wraps inside the bounded chip, keeping the @ and robot glyph visible
Capture provenance

Rendered by the Flutter widget engine in a flutter test run (production widgets, production theme; no device or simulator). Before: this PR's declared base 3c7f288c60d67df78577b237e27c3dfc8831aaa1. After: its head 39afd73b0adfde14164f4b10dbd089cb498312b6.

@loganj
fix(mobile): bind same-name mentions to exact selected identities
8489373 
Signed-off-by: Logan Johnson <loganj@squareup.com>
@github-actions

github-actions Bot commented Sep 5, 2026 •
edited
Loading

Copy link
Copy Markdown

🔐 Codex Security Review

Status: review required for the current range.

The current range is 3c7f288c60d67df78577b237e27c3dfc8831aaa1...39afd73b0adfde14164f4b10dbd089cb498312b6.
A new review must complete for this exact range. When manual authorization
is required, a Block organization member must comment exactly
@buzz-security-review 39afd73b0adfde14164f4b10dbd089cb498312b6 to authorize a new review.
Any previous review applies only to its recorded range.

@loganj loganj mentioned this pull request Sep 5, 2026
Open
@loganj
fix(mobile): scope exact-label semantics to mention text
4e0054f 
Signed-off-by: Logan Johnson <loganj@squareup.com>
@loganj
test(mobile): preserve full-key mention accessibility
2ac8764 
Signed-off-by: Logan Johnson <loganj@squareup.com>
@loganj
fix(mobile): keep unbound qualified mentions non-notifying
87d7807 
Signed-off-by: Logan Johnson <loganj@squareup.com>
@loganj
fix(mobile): wrap exact mention chips at accessible text sizes
39afd73 
Signed-off-by: Logan Johnson <loganj@squareup.com>
@loganj

loganj commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

@buzz-security-review 39afd73

@loganj

loganj commented Sep 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Independent exact-mention readiness evidence — 2026-09-07

Verified current range: main at 3c7f288c60d67df78577b237e27c3dfc8831aaa1 → 39afd73b0adfde14164f4b10dbd089cb498312b6; +483/−47 = 530 across 10 files. Still OPEN/draft. Reconciliation only — no source changes, pushes, or test reruns.

  • Local, exact final head (isolated per-PR TMPDIR and worktree-local targets): just mobile-check exit 0; full just mobile-test exit 0 (2,081 passed); full just ci exit 0 (Rust run-tests, Desktop 6,450 JS tests, Tauri tests, desktop/web builds, full mobile suite). Receipts: WORK_LOGS/MENTIONS_REVIEW_A24EBE91/7385/final/ (results.txt: mobile-check=0, mobile-test=0, ci=0).
  • Repaired defects at this head carry red→green evidence: unbound qualified full-key namesake send/render (121 focused tests) and the 320px/200% chip overflow fixed by wrapping only the label in Flexible (all rendered-mention tests).
  • Remote: CI run 34137039823 completed success at this exact head. Head rollup: 32 success / 31 intentional path-filtered skips; nothing pending or failing.
  • Chip regression parity (Desktop fix(desktop): restore mention chip identity icons #7338): merged fix(desktop): restore mention chip identity icons #7338 "restore mention chip identity icons" (4afef864) restored the missing @/robot glyphs after fix(desktop): bind duplicate mention selections to exact recipients #7133. On this exact head the mobile rendered-chip matrix — 36 cases: ordinary/full-key-qualified human/agent × 320/240/160 px × 100/150/200% text scale — passed 39/39, asserting glyph containment inside the chip, full-label semantics, and exact 64-char key taps; removing the label-only Flexible reproduces a 214 px RenderFlex overflow at 320px/200% (red mutant), and the restored head passes. Evidence: WORK_LOGS/CHIP_ACCEPTANCE_4D364726/.
  • Advisory security: the authorized exact-head request (comment) ran as run 34139063516; its "Run Codex Security Review" job exceeded the 40m0s maximum execution time and was cancelled — post skipped, no findings produced. That is an external advisory-workflow limitation, not a security clearance and not a failed PR test. No rerun was requested in this reconciliation.
  • The PR body's composed-stack reference (verify/mobile-combined-20260905 at 40630565) predates the fix(mobile): bind same-name mentions to exact selected identities #7385 repairs (2ac8764b → 39afd73b); it is stale relative to this head and is not proof of the repaired branches. No new composed-tree testing is claimed; the reference branch was left unchanged.
  • Widget evidence only: no iOS/Android device or simulator was available; this is not native-device acceptance. Draft retained; no merge requested.

2026-09-07 update: the stale composed reference described above has been retired — the verify/mobile-combined-20260905 remote branch at 40630565 was deleted and the composed-stack sections were removed from all 11 PR bodies (no replacement combined-testing claim was made). This comment's 2026-09-07 evidence statements are preserved as historical record. Receipt: WORK_LOGS/COMPOSED_REFERENCE_RETIREMENT_A4B214BD/REPORT.md.

@loganj
loganj marked this pull request as ready for review September 8, 2026 13:54
@loganj
loganj requested a review from a team as a code owner September 8, 2026 13:54
jedwards27
jedwards27 requested changes Sep 8, 2026
View reviewed changes

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent — REQUEST CHANGES on exact head 39afd73b0adfde14164f4b10dbd089cb498312b6 (base 3c7f288c60d67df78577b237e27c3dfc8831aaa1).

Two exact-identity failures remain:

  1. Persisted drafts discard selected identities. ComposeBar keeps selection ownership only in its in-memory mentionMap (mobile/lib/features/channels/compose_bar/compose_bar_widget.dart:219-226), while ComposeDraft persists/restores text only (mobile/lib/features/activity/compose_drafts_provider.dart:18-40,104-143). After teardown/restart, sending reconstructs recipients from current member names (compose_bar_widget.dart:486-503). Two independent production-widget reproductions selected same-name recipients, unmounted/remounted against the persisted store, and then found Send never reached onSend: the restored plain @Scout had become ambiguous. The UI preserves exact-looking text while silently losing authored identity intent.

    Required: persist and hydrate exact bindings atomically with draft text in this PR (or absorb the relevant stacked persistence work), and add teardown/restart → restore → send coverage including profile/roster changes and malformed/legacy state. A follow-up PR does not make this merge commit safe.

  2. Renaming the unqualified recipient retargets their rendered chip. renderedMentionBindings derives aliases from current profile names (mobile/lib/shared/mentions/mention_bindings.dart:67-70) and uses the qualified occurrence only to subtract its key (:98-109); it never assigns the remaining tagged identity to the historical unqualified occurrence. For authored @Scout @Scout (<second key>) with signed recipients {first: "Renamed Scout", second: "Scout"}, an executable adversarial test expected the plain chip to bind first but observed it bind second, so both visible chips target the second person. Signed p tags remain correct, but tapping the first mention opens the wrong profile, contradicting the PR’s stated rename behavior.

    Required: reconstruct unqualified occurrence ownership deterministically from event-tagged identities after exact qualified labels claim their keys. Add production-widget tap regressions for each recipient renaming independently, reversed tag order, missing profiles, and ambiguous leftovers.

Validation at the pinned head:

  • just mobile-test — 2,081 passed, exit 0, clean tree before/after.
  • Focused mention/content/composer tests — 122 passed, exit 0; a production-call mutation was caught by the relevant selection test.
  • git diff --check clean; required CI green.

Confidence gap, not additional author action: no native iOS/Android keyboard, VoiceOver/TalkBack, theme, or navigation journey was observed. Exact-head advisory security automation produced no verdict; required CI is green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jedwards27 jedwards27 jedwards27 requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@loganj @jedwards27